Skip to content

Conversation

vincentmacri
Copy link
Member

@vincentmacri vincentmacri commented Oct 4, 2025

Mostly done with ruff and autotyping, but also some manually for the main function field classes.

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

⌛ Dependencies

@vincentmacri vincentmacri changed the title Code cleanup and typing annotations for function fields PEP8 and typing annotations for function fields Oct 4, 2025
@vincentmacri vincentmacri requested a review from kryzar October 4, 2025 00:29
Copy link

github-actions bot commented Oct 4, 2025

Documentation preview for this PR (built with commit 140807b; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

Copy link
Contributor

@fchapoton fchapoton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, but please next time:

  • do no make any change about spaces around arithmetic operators ;
  • rather do only annotations changes, when changing so many files
  • I would also prefer to avoid Literal for the moment

@vincentmacri
Copy link
Member Author

vincentmacri commented Oct 6, 2025

Thanks for the review!

* do no make any change about spaces around arithmetic operators ;

I can certainly do that, but can I ask why you don't like the spaces around arithmetic operators? Do you not like spaces around operators, or is it just too annoying to review for how minor of a fix it is?

* rather do only annotations changes, when changing so many files

For sure! Originally I was not planning to change this many files but I guess I got carried away.

* I would also prefer to avoid Literal for the moment

Sure. There aren't many places where we can use it anyway since it only works with Python literals. Something like Literal[Integer(1)] doesn't work. So the only places where Literal could theoretically be used are the handful of functions that use int instead of Integer, functions that return a constant bool, or maybe for things like an algorithm parameter that has a handful of valid string options.

The reason I wanted to use Literal was I was hoping that Cython would be smart enough that something like this would be optimized out, but after some testing it seems Cython is not capable of this (at least not yet, maybe one day):

def constant_false() -> Literal[False]:
    return False

The hope was that if a Cython file imported constant_false from a Python module and then tried something like this:

if constant_false():
    assert False

that the if statement would be optimized out. Unfortunately, at least as far as I can tell, it isn't currently optimized out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants